Open external links in frameset

Hi Jason, sure this sounds like a desirable feature. Though it's not really an IFRAME is it? It's just a frameset with the bottom frame containing the linked page. Perhaps we could make a radio list which chooses the behavior for external links:

( • ) Open in same window
( • ) Open in new window
( • ) Open in split frame

Split Frame Contents:
[ Text area for the contents of the iFrame ]

Split Frame Height: [ 200 ]
Enter frame height in pixels.

Provide a menu hook for the iFramed page. So the user wouldn't ever need to actually create an HTML page defining the frameset, it could be provided by a theme_ function. Anyway... it sounds like a neat idea. Post a patch and I'll review. Thanks!

Good idea, any news, release or patch , thanks

This looks like a great idea! I know little to nothing about how one would execute this, though I am more than willing to provide testing and anything else anyone brave enough to take this on might need. Let me know.

Subscribing

Anyone willing to give this a try?

-Victor

Wish I knew how to do this - it'd be first on my list, I know.

I am willing to chip in $$ for this. So if anyone is willing to take a bounty let me know.

Thanks,
Victor

I’d be willing to take this on.

I’ve had a look at the module, and made a couple of minor patches.

Are Victor and the maintainers happy with this?

Sorry, forgot to mention that patches are for 6.x. I’ve left this issue at 5.x because I guess that is what is needed, but I will do 5.x & 6.x.

Also can this code be removed from 6.x now that jQuery 1.2.3 is default?

Hi zeta! Thanks for the *excellent* set of patches! They all look great, however, let's tackle all those in separate issues and keep discussion here focused on the frameset problem. I'll comment further on your patches once they're in separate issues. I really, really appreciate the patches, I just don't want this to become a general purpose forum for fixes in Link module.

Zeta,

Thanks for the quick turn around on this. Unfortunately I am not a programmer so I will stay away from commenting on the patch, looks like QuickSketch is doing a good job validating it.

Can you possibly send me a link where I can see this patch in action? My needs are for 5.x . Also what would appear on the top frame? Can the data for the top frame be shared with the header of the theme (I am using Garland)?

Thanks,
Victor

Hi Victor,

Well it should do what you want it to do ;-) ... it’s not finished yet. The above are some contributions while I was looking at what would be involved.

I’m now working on the actual issue itself. If we compare with the other options;-

• replaces the current page with the external page
• leaves the current page, and opens the external page in another window / tab

I thought the new option would be;-

• places the current page in top part of a frame with the external page in the lower part

So it would be possible to also provide;-

• places the header in top part of a frame with the external page in the lower part

This will be more difficult, as it involves extracting just the header, but it is possible.

Nick

I’ve got a setup which can do the following;

• output a frameset
• modify external links to open the frameset and load the current page in one frame and the external page in the other
• modify external links to open the external page in the other frame without reloading the rest

Unfortunately, I havent yet managed to get jQuery to detect whether its page is in a frameset or not, so modifying external links is one or the other at the moment.

I’ll set it so that you can open a frameset with a custom link to frame/int_path/--/ext_path then all external links in the pages open in the frame replacing ext_path.

Will post a patch…

Initial patch against -6--1 Does it also apply against 5-1 ?

Found a jQuery plugin that helps with cross-frame DOM, but might be overkill for detecting page is in frameset.

I have now managed to get jQuery to detect whether its page is in a frameset or not.

If it is in a frameset, external links will open the external page in the other frame without reloading the rest.
If not, external links will open the frameset, and load the current page in one frame and the external page in the other.

This patch is against -6--1: Does it also apply against 5-1 ? If not, I’ll do another patch.

A couple things here:

1. When using the open in frameset option, external links will always go to frame/node/--/[link url without http://] for me. This happens in both Safari and Firefox. Are you using a different browser?
2. The frameset should be in a theme function since it outputs HTML.
3. I think the original request of this was to make an About.com style exit. Where a small header representing the original site is shown at the top. The content inside of this frame would likely need to be another theme function.

Anyway, the primary goal of opening in a frameset still isn't working. Don't worry about a 5.x patch I can port it once this is working.

1. Sorry, missed that change when I was editing the patch file! My dev env is setup better now.
2. Still working out how to do that. I was following advice in http://api.drupal.org/api/function/page_example_foo/6 to just print the html.
3. I don't know if www.lime.com or about.com has been re-designed, but I can’t get either to open a frame, pop-up or anything except a new page. Can do this even more simply; but need to work out 2. first.

Patch includes my attempt at theme function, but it doesn’t return anything … yet.

This was a patch I cooked up awhile back when I saw Victor's message on Drupal's forum. I had not received a reply from Victor at the time and was just trying it out. When Victor's reply came only found out that a patch has been posted by Zeta. Nonetheless, I'm posting the patch here and hopefully it can complement what Zeta is doing.

Features are as outlined by quicksketch in #1; patch is against 5.x-1.x-dev

OK, I’ve worked out the problem with theme functions in D6 – I had module_hook where it should have been hook …

You can now choose an iframe, surrounded by your theme; header, footer and other blocks that can be controlled in admin as usual.

Or an old-fashioned frameset with the referring page in the top frame and the external page in the bottom frame by default. External links in the referring page open in the bottom frame, replacing the existing content.

Both options themeable with template files. Let me know if this is anything like you were expecting.

@Nate Thanks for looking after this, let me know if porting to D5 is not trivial.

@edkwh Sorry for any duplication; I’ve looked at your patch, but I’ve used a slightly different method to get the header etc. around the external content (hint cf. print and return in theme functions).

Guys, I am still very interested in this feature. Let me know what I can do help you all (testing?).

-Victor

I tried to run the patch on extlink-6.x-1.6.tar and got this error:

Macintosh:extlink adam$ patch -b < ext_frame-6--1_1.patch
patching file extlink.js
Hunk #1 succeeded at 65 with fuzz 2.
patching file extlink.module
Hunk #2 FAILED at 33.
patch: **** malformed patch at line 135: Index: extlink/frameset.tpl.php

I patched the version from

Macintosh:~ adam$ cvs -z6 -d:pserver:anonymous:anonymous@cvs.drupal.org:/cvs/drupal-contrib checkout contributions/modules/extlink

and got the same error.

I applied the patch manually (I may have made a mistake) and my external urls go to:

http://localhost/iframe/www.google.com

which comes back as not found.

Adam

Wow this patch has a lot of crazy (as in neat) functionality. Overall it's looking really good and well written. It's great to see some custom .tpl.php files in Drupal 6! Oh how we love the new theming system :)

Comments:
- The .tpl.php files and menu paths should all begin with extlink, so we don't have name-space conflicts with other modules (iframe module? Who knows...)
- The default frameset.tpl.php files could probably provide a better default for the page titles, printing out the title of the site would be good, we definitely don't want to put "Drupal" right in the title of the page by default.
- The .tpl.php files need a description of all the variables that are made available to them by default. See any of the .tpl.php files provided by core modules for a good example of documenting .tpl.php files.
- For options, I didn't understand what the difference was between "A new frame (in a page)" and "A new frame (split window)". Could we change these to "In an iFrame" and "In a Frameset"

Functionally, this all works great. I'm most afraid of new support requests it will generate regarding "How do I customize this to do X" questions. We can minimize this significantly by providing a plethora of documentation in the code.

After this patch is finished, we can polish it with a little bit of additional JavaScript for the administration page. We should hide the options for "Height of frame for external links:" and "Width of frame for external links:" options if their corresponding option is not selected.

Thanks for your enthusiasm :-) I’ve addressed all your comments, so this should be much better.

Removed code for internal links, and protocol.

Added some comments to explain what’s going on.

Looks good enough to me for an initial commit. The important part is that the .tpl.php is in it's final form (looks good now).

I've got a small concern about the URL structure, especially using '--' to separate the URL. Is there a limitation somewhere that we couldn't use a GET parameter? a la ?url=[encoded url]?

Regardless, I'm fine with committing as-is for now. We do still need to port to 5 though.

Will you post here when you commit so people can download the cvs version?

version using $_GET[]’s to do the parsing.

I know I attached this…

I applied the patch on 5.x.1.6. But not sure what to put in configuration page, frame content text box. The open in split frame leaving the box empty does not open the external links at all.

Which patch? the one with a 6 in the name?

The one in #19 for 5.x

Can this be implemented as a new release?

is exist any correct patch for D5

Oh! would love to see this feature on a 6.x release. :-)

I second that. Would love to see a new 6.x release. thanks.

subscribe

Add me to the list that would love to see this for 6.x.

Again, why is this still not committed into 6.x?

At the time, the Drupal 5 module was still supported so the two modules were maintained in sync. Now I think it's best to just let Drupal 5 go its own way and stop adding any new features to that branch. Now it's a matter of rerolling this patch (it doesn't apply any more) and giving it another review.

Subscribing

Subscribing. Not sure what's going on here, but it would be nice to have something to work from on this.

Subscribing. Interested in the D5 patch version

Subscribing. May be interested in a 6.x version of this in the near future; if so will look at porting that patch.

Hi Zeta,
Im using drupal core 6.20. Im getting the following error when i applied ur patch. What should i do to overcome this issue?
$ patch < ext_frame-6--1_.patch
patching file extlink.js
patch: **** malformed patch at line 14: \ No newline at end of file

Im new to drupal. For which version of release, this patch is working? I have tried with drupal core 6.20 and extlink-6.x-1.11. Is that will work correctly? Pls reply anybody..

I’ll try it on External Links with D6.20 now (as you can see, the patch is 2½ years old).

Don’t know why this ever worked O_o maybe patch utility has been updated?

Extlink was version 6.x-1.6 at the time and is now 6.x-1.11, so this needs a re-roll at least, maybe its not even needed anymore (quicksketch did say he was fine with committing as-is at one point).

This is a fair bit of work – 8k patch, mostly additions, to an 11k code-base – so please let me know if it won’t be committed, due to the game has changed, bloated feature or whatever.

This is a fair bit of work – 8k patch, mostly additions, to an 11k code-base – so please let me know if it won’t be committed, due to the game has changed, bloated feature or whatever.

All told, I've had second thoughts about this feature. This module has over 20,000 installations, and about 10 people have specifically requested this functionality. It's also a huge amount of code in comparison to the module itself, and I fit into the 90% who would not need this functionality, making it more likely to break in the future.

Some other obscure features have come back to bite me in other projects (Webform, Fivestar, Flag) that made me wish they had not been added or separated into a different module. I think that this particular request fits into that category also, and I'd be much happier to see this functionality in a different project than in extlink.

It's also a huge amount of code in comparison to the module itself

Precisely, and I also fit in the 90%.

Looking at other ‘external link’ modules, they all seem a lot lighter – and might want to keep it that way.

Maybe, having started this, I could create yaextlink™ to see how big that 10% is.

Do you want to ‘won't fix’ this?

Yep, think so. Sorry for all the troubles zeta, I know you were quite earnest originally when we were talking about this initially.

Hi Zeta & Quicksketch,
Thanks for ur replies.. I was eagerly awaited for the patch but im totally disappointed becoz this functionality is most important for my site.
Am developing a news aggregator site which gets rss feed links from other popular sites and displays in the site (http://news.anytime.in). I would like to open those links in a frameset page with my site header in top frame and external page in bottom frame.
Is there anyother ways to include frameset in drupal?? Pls suggest any solution..

Hi ssvraja,

Looking at building a new module that would use this code to extend existing module. Don’t want to start a completely separate module as that’s not the way to go.

New patch for 6.1.11 for those who are still playing with this.

subscribing.